Skip to content

feat(skills): add skill editors #5705

Open
icecrasher321 wants to merge 27 commits into
stagingfrom
staging-v2
Open

feat(skills): add skill editors #5705
icecrasher321 wants to merge 27 commits into
stagingfrom
staging-v2

Conversation

@icecrasher321

@icecrasher321 icecrasher321 commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds way to set skill editors so only certain people can edit skills. Derived access to skills applies (so workspace, org admins are always skill editors for all skills in the workspace.

Type of Change

  • New feature

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

icecrasher321 and others added 4 commits July 16, 2026 00:50
…t staging

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts:
#	scripts/check-api-validation-contracts.ts
Same DDL as the dropped 0261 (skill_member table, enums, indexes, skill.workspace_shared)
plus the hand-written write-user backfill, renumbered after staging's 0261.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 16, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Jul 24, 2026 1:15am

Request Review

icecrasher321 and others added 2 commits July 16, 2026 12:12
# Conflicts:
#	apps/sim/app/api/mothership/execute/route.ts
#	apps/sim/executor/handlers/agent/skills-resolver.test.ts
#	apps/sim/lib/copilot/chat/payload.ts
#	apps/sim/lib/copilot/chat/process-contents.test.ts
#	apps/sim/lib/copilot/chat/process-contents.ts
#	apps/sim/lib/copilot/vfs/workspace-vfs.ts
#	apps/sim/lib/mothership/inbox/executor.ts
#	apps/sim/lib/mothership/skills.test.ts
#	apps/sim/lib/mothership/skills.ts
#	packages/db/migrations/meta/0262_snapshot.json
#	packages/db/migrations/meta/_journal.json
#	scripts/check-api-validation-contracts.ts
Staging claimed 0262 (strong_storm); same DDL plus the hand-written
write-user backfill, renumbered on the merged snapshot chain.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
icecrasher321 and others added 5 commits July 17, 2026 22:26
…t staging

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts:
#	scripts/check-api-validation-contracts.ts
Staging claimed 0263 (workflow_fork_sync_excluded); same DDL plus the
hand-written write-user backfill, renumbered on the merged snapshot chain.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@icecrasher321
icecrasher321 marked this pull request as ready for review July 18, 2026 21:13
@cursor

cursor Bot commented Jul 18, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Skill create/update/delete authorization changes affect all workspace skill mutations and member removal now deletes editor grants; behavior is intentional but worth regression-testing non-editor writers and admin member removal.

Overview
Introduces skill editors: workspace members can see and run every skill, but only explicit editors (plus workspace admins) can edit, delete, or manage the editors list. New GET/POST/DELETE on /api/skills/[id]/members handles the roster with audit/analytics; skill POST/DELETE now enforce editor access for updates while creation still needs workspace write, and the creator gets an editor row on insert.

Skills UX moves off the gallery modal onto dedicated routes (/skills/new, /skills/[skillId]) with Share, unsaved-changes guards, and a Skill Editors section. Legacy ?skillId= links redirect to the detail page. Import drops the GitHub fetch API and modal GitHub flow in favor of shared .md/.zip handling; name conflicts surface as 409 inline errors.

Shared permissions chrome (AddPeopleModal, MemberRow, role options) replaces duplicated credential “add people” UI and backs skill sharing. Leaving a workspace (user or admin APIs) now clears skill_member rows via removeWorkspaceSkillMembershipsTx.

Custom tools in workspace settings get a deep-linkable detail sub-view (custom-tool-id) and extracted schema/code editor pieces (wand generation, validation) aligned with the canvas modal. RichMarkdownField grows with content on full pages and only self-scrolls when maxHeight is set.

Docs add Skill Editors, Chat / skills, and updated FAQs. Mothership execute reuses assertActiveWorkspaceAccess for permission instead of a separate lookup.

Reviewed by Cursor Bugbot for commit b80502d. Configure here.

@greptile-apps

greptile-apps Bot commented Jul 18, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds per-skill permissions and a new skill management page. The main changes are:

  • Admin and Member roles with workspace-derived access and explicit deny markers.
  • Permission checks across APIs, workflows, Copilot, VFS, and agent execution.
  • Member-management APIs, hooks, shared controls, and a full-page skill editor.
  • Database schema changes and an idempotent access backfill.

Confidence Score: 4/5

Agent execution can bypass skill permissions when its enforcement flag is false or absent.

  • API and membership paths consistently apply the new permission model.
  • The agent resolver treats a missing enforcement scope as unrestricted access.
  • Restricted skill content can reach affected non-interactive runs.

apps/sim/executor/handlers/agent/skills-resolver.ts and its execution callers

Security Review

Skill checks fail open when agent execution omits or disables the credential-enforcement flag. Affected runs can load restricted skill metadata and content.

Important Files Changed

Filename Overview
apps/sim/lib/skills/access.ts Adds effective role resolution, explicit grants and denies, member listing, update checks, and workspace-removal cleanup.
apps/sim/app/api/skills/[id]/members/route.ts Adds authenticated member listing, grants, role changes, removals, and transactional last-admin protection.
apps/sim/app/api/skills/route.ts Adds skill visibility and administrator checks to list, update, and delete operations.
apps/sim/executor/handlers/agent/skills-resolver.ts Makes skill permission filtering conditional, leaving executions without the enforcement flag unrestricted.
packages/db/migrations/0264_tough_steel_serpent.sql Adds skill sharing and membership storage with an idempotent access backfill.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Agent execution] --> B{actor.enforce is true}
  B -- Yes --> C[Load user skill access]
  C --> D[Filter skills with canUseSkill]
  B -- No or unset --> E[Return no access context]
  E --> F[Skip skill permission checks]
  F --> G[Restricted skill content can load]
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
flowchart TD
  A[Agent execution] --> B{actor.enforce is true}
  B -- Yes --> C[Load user skill access]
  C --> D[Filter skills with canUseSkill]
  B -- No or unset --> E[Return no access context]
  E --> F[Skip skill permission checks]
  F --> G[Restricted skill content can load]
Loading

Reviews (1): Last reviewed commit: "fix lint" | Re-trigger Greptile

Comment thread apps/sim/app/api/skills/[id]/members/route.ts Outdated
Comment thread apps/sim/executor/handlers/agent/skills-resolver.ts Outdated
icecrasher321 and others added 3 commits July 22, 2026 10:30
…t staging

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts:
#	apps/sim/executor/handlers/pi/pi-handler.ts
#	apps/sim/lib/copilot/vfs/workspace-vfs.ts
#	scripts/check-api-validation-contracts.ts
Staging claimed 0264 (fat_ikaris); same DDL plus the hand-written
write-user backfill, renumbered on the merged snapshot chain.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@icecrasher321 icecrasher321 changed the title feat(skills): permissions layer feat(skills): add skill editors Jul 22, 2026
icecrasher321 and others added 4 commits July 23, 2026 00:11
…t staging

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
# Conflicts:
#	apps/sim/executor/handlers/agent/skills-resolver.test.ts
#	apps/sim/lib/workflows/skills/operations.test.ts
#	scripts/check-api-validation-contracts.ts
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e-check

better-auth 1.6.23 and fumadocs-mdx resolve ^4.3.6 to a nested zod 4.4.3,
which makes @sim/auth's inferred betterAuth types non-portable (TS2883) and
split docs onto a second zod instance. Both ranges accept the repo-wide
pinned 4.3.6, so a single hoisted copy satisfies everything.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Moves the rich-markdown and custom-tool editing surfaces out of modals and
onto full-page surfaces, and collapses the duplicated chrome behind shared
components.

Skills
- Add /skills/new, a full-page create surface mirroring the skill detail page
  (CredentialDetailLayout + DetailSection + unsaved-changes guard). "Add to
  Sim" navigates there instead of opening a modal.
- Import moves to a header action (SkillImportButton) backed by a shared
  readSkillFile helper; the GitHub-URL import and its /api/skills/import route
  are removed.
- Skill name validation is now one shared validateSkillName, replacing three
  copies of the kebab-case rule and its messages.
- The skill editor roster renders through the shared MemberRow instead of
  re-deriving its identity block, with a locked role control and a lock-reason
  tooltip explaining inherited workspace-admin access.

Custom tools
- Extract the canvas modal's schema/code editors into a shared
  custom-tool-editor module (fields, wand generation, schema helpers), cutting
  custom-tool-modal.tsx by ~900 lines.
- Settings > Custom tools gains a full-page detail sub-view (SettingsPanel +
  SettingsSection + saveDiscardActions), deep-linkable via ?custom-tool-id.
  Rows are clickable; delete now lives only in the detail view.
- Replace legacy Button/Input/Badge/Label with the chip family, move chip-field
  chrome into CodeEditor behind an error prop, and delete its dead wand button.

Rich markdown field
- maxHeight is now opt-in: omit it on a page and the editor grows with its
  content so the page owns the only scrollbar. Modals pass explicit caps.
- The field variant drops to font-weight 400 to match adjacent chip fields.
@waleedlatif1

Copy link
Copy Markdown
Collaborator

@greptile

@waleedlatif1

Copy link
Copy Markdown
Collaborator

@cursor review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 3 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit b80502d. Configure here.

if (isSkillNameConflictError(error)) {
toast.error(
`"${skill.name}" already exists in this workspace but isn't shared with you — ask a skill admin for access`
)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong duplicate skill error text

Medium Severity

On HTTP 409 from creating a suggested skill, the toast says the skill exists but is not shared and to ask a skill admin. Workspace members already see and use every skill; 409 only means the name is taken, not missing access.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b80502d. Configure here.

})
setErrors({})
const createdId = created[0]?.id
router.push(createdId ? `${skillsHref}/${createdId}` : skillsHref)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Create navigates to wrong skill

Medium Severity

After a successful create, navigation uses the first element of the upsert response array. That list prepends built-in template skills, so the user is often routed to a built-in detail page instead of the skill they just created.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b80502d. Configure here.

description: 'Added skill editor',
metadata: { targetUserId: userId },
request,
})

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Duplicate editor add logs audit

Low Severity

When two requests race to add the same editor, onConflictDoNothing can skip the insert while the handler still emits skill_shared and SKILL_MEMBER_ADDED audit, recording an add that did not happen.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit b80502d. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants